Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] Regressions in .env loading #11823

Closed
jpetazzo opened this issue May 15, 2024 · 4 comments · Fixed by #11824
Closed

[BUG] Regressions in .env loading #11823

jpetazzo opened this issue May 15, 2024 · 4 comments · Fixed by #11824
Assignees
Labels

Comments

@jpetazzo
Copy link

Description

The way .env files are loaded has changed twice recently.

Let's assume the following project structure:

$ tree -a
.
├── .env
├── compose.yaml
└── sub 
    └── .env

When running docker compose in the sub/ directory, ...

  • up to Compose 2.23.3, Compose merges both .env files, with the top-level .env file taking precedence over the sub-level one
  • from Compose 2.24.0 to 2.24.5, Compose only uses the .env file in the current directory, and ignore the one at the top level
  • from Compose 2.24.6 to at least 2.27.0, Compose only uses the .env file at the top-level directory, and ignores the one in the sub directory

Both changes broke some folks' workflows in different ways (see #11531 and #11575). The second change might be related to #11405 but I haven't looked at the code, so I might be wrong.

Steps To Reproduce

cd /tmp
mkdir -p envtest envtest/sub

cat >envtest/compose.yaml << "EOF"
services:
  echo:
    image: alpine
    command: echo $ROOT $SUB win=$WIN
EOF

cat >envtest/.env << EOF
ROOT=root
WIN=root
EOF

cat >envtest/sub/.env << EOF
SUB=sub
WIN=sub
EOF

cd envtest/sub
docker compose run echo

Here is the output with various versions of Compose...

Up to Compose 2.23.3:

root sub win=root

From Compose 2.24 to 2.24.5:

WARN[0000] The "ROOT" variable is not set. Defaulting to a blank string.
sub win=sub

From Compose 2.24.6:

WARN[0000] The "SUB" variable is not set. Defaulting to a blank string.
root win=root

Compose Version

N/A

Docker Environment

N/A

Anything else?

The ability of loading a .env file from the current directory (instead, or in addition to, the top-level .env file) is very powerful.

Example user story: I develop an application stack made of 2 services, service1 and service2. I sell that stack as a SAAS to multiple customers. Each customer gets their own stack, with their own instance of each service. I want to deploy each stack with Compose and follow "DRY" principles: I want to avoid repeating or duplicating code as much as possible.

I can achieve that with the following structure:

.
├── compose.yaml
├── customers
│   ├── customer1
│   │   └── .env
│   ├── customer1
│   │   └── .env
│   └── customer3
│       └── .env
├── service1
│   └── Dockerfile
└── service2
    └── Dockerfile

The .env files in the customerX directories set customer-specific information (credentials, plan limits, etc.) and they also set COMPOSE_PROJECT_NAME to a unique value.

That way, to manage the stack for customerX, all I have to do is go to their specific directory and run docker compose command. I don't need to set environment variables - and more importantly, I don't risk forgetting to set a critical environment variable or command-line flag before manipulating a customer environment.

@ndeloof
Copy link
Contributor

ndeloof commented May 16, 2024

This topic is a never ending issue it seems :'(
Any attempt to "fix" it comes with some users being frustrated by their usage being broken.

As .env file is loaded from "project directory" (base of your compose file) and compose looks for a compose file in the parent folder(s) there's no reason sub/.env would be loaded. But we also allow COMPOSE_FILE to be declared .. by a local .env file, so this one has to be parsed before we actually know project directory - a chicken vs egg problem here. This is the reason sub/.env used to be included in the interpolation, but it never has been documented we "merge" both sub/.env and <project directory>/.env - and obviously some started relying on this 😓

For your scenario, I would recommend to use an explicit --env-file flag to select customer-specific configuration, so you don't rely on this "feature"

@jpetazzo
Copy link
Author

I don't understand; it looks like the original behavior (read both .env files and merge) satisfied everyone (by fitting to both use-cases: "I want a global .env at the top-level" and "I want a more specific .env in a subdirectory").

Furthermore, the original behavior did not have the chicken-and-egg problem that you describe, since it was possible to:

  1. read the local .env
  2. set the COMPOSE_FILE if necessary
  3. look for the COMPOSE_FILE, walking up the directory tree if necessary, until it's found
  4. read the global .env if it's in a different directory)

Or am I missing something?

The current behavior (read only the top-level .env) does have a chicken-and-egg problem indeed, since the .env might set the COMPOSE_FILE which might make the top-level directory irrelevant. But that problem didn't exist with the original implementation 😅

@ndeloof
Copy link
Contributor

ndeloof commented May 16, 2024

Actually compose v1 only used the local .env file and ignored the one in project directory. This used to bring some issues (as all other paths are relative to project directory) so we changed this, which brings some more issues
Still "load local .env" was required so users can set COMPOSE_ variables - but there's no other reason we should load both env file, until we decide to introduce this as an explicit, documented, tested feature. I agree this covers your usage very well, I just wonder about impacts as this area already suffered many regressions.

@ndeloof
Copy link
Contributor

ndeloof commented May 16, 2024

Also to be mentioned: the logic you describe (read the local .env, then read the global .env if it's in a different directory) won't give you what you expect, as latest read would then set WIN=root, while you're expecting is an environment override. This used to work as the whole local .env was set as os.Env

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants